OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant #22785

richardleach · 2024-11-26T14:16:52Z

This commit adds OP_SUBSTR_NIBBLE and associated machinery for fast handling of the constructions:

    substr EXPR,0,LENGTH,''

and

    substr EXPR,0,LENGTH

Where EXPR is a scalar lexical, the OFFSET is zero, and either there is no REPLACEMENT or it is the empty string. LENGTH can be anything that OP_SUBSTR supports. These constraints allow for a very stripped back and optimised version of pp_substr.

The primary motivation was for situations where a scalar, containing some network packets or other binary data structure, is being parsed piecemeal. Nibbling away at the scalar can be useful when you don't know how exactly it will be parsed and unpacked until you get started. It also means that you don't need to worry about correctly updating a separate offset variable.

This operator also turns out to be an efficient way to (destructively) break an expression up into fixed size chunks. For example, given:

my $x = ''; my $str = "A"x100_000_000;

This code:

$x = substr($str, 0, 5, "") while ($str);

is twice as fast as doing:

for ($pos = 0; $pos < length($str); $pos += 5) {
    $x = substr($str, $pos, 5);
}

Compared with blead, $y = substr($x, 0, 5) runs 40% faster and $y = substr($x, 0, 5, '') runs 45% faster.

This set of changes requires a perldelta entry, and I will add one shortly.

leonerd

A couple of small comments but overall nothing troubling-looking here.

I wonder a bit about the name though. I've usually seen the word "nibble" to mean a half-byte; i.e. a 4-bit value. I wondered if that is what is going on here at first. If there are other candidate names to call it, perhaps something else would be better? Not a huge problem though.

peep.c

pp.c

richardleach · 2024-11-26T22:55:31Z

How about substr_chop, since a big part of its design is being a fast route for getting to Perl_sv_chop?

Food related alternatives: substr_peck? substr_graze? substr_smidgen? substr_julienne? substr_shuck? substr_heel, like the end of a loaf? Non-foodie: substr_shave?

richardleach · 2024-11-26T23:14:30Z

Not sure what's going on with the ABRT test failures. Don't get them locally. ./perl -Ilib t/perf/benchmarks.t seemingly only uses 8MB of memory, so running out of memory doesn't seem to be the cause.

richardleach · 2024-11-27T23:41:30Z

Not sure what's going on with the ABRT test failures.

Looks like an op_private flags assertion. I'll dig into it soon.

richardleach · 2024-11-28T22:50:12Z

I'm rebasing and renaming it to SUBSTR_CHOP.

leonerd · 2024-11-28T23:08:02Z

Doesn't perl's chop() function eat from the other end though?

richardleach · 2024-11-28T23:15:40Z

The Perl chop takes from the end, but Perl_sv_chop takes from the front. (I don't know who we have to thank for that amazing piece of naming.) The pp_ function for this op calls Perl_sv_chop.

jkeenan · 2024-11-29T00:38:28Z

@richardleach , merge conflicts ^^

leonerd · 2024-11-29T10:03:06Z

The Perl chop takes from the end, but Perl_sv_chop takes from the front. (I don't know who we have to thank for that amazing piece of naming.) The pp_ function for this op calls Perl_sv_chop.

Oh wow. Huh. In that case, might as well call this one SUBSTR_CHOP indeed then.

Otherwise my thoughts were going to be something like SUBSTR_PREFIX but that isn't much more descriptive.

Grinnz · 2024-11-30T10:22:18Z

Consider ltrim, with inspiration from PHP and Redis (or lstrip a la Ruby/Python but that sounds more whitespace-specific). Though it is also unrelated to builtin::trim, I think it's a bit more descriptive at least

richardleach · 2024-12-01T22:50:10Z

Consider ltrim, with inspiration from PHP and Redis (or lstrip a la Ruby/Python but that sounds more whitespace-specific). Though it is also unrelated to builtin::trim, I think it's a bit more descriptive at least

Hmmm, I'm not sure about this. It seems only more descriptive to someone who already is familiar with ltrim, otherwise it's likely to lead to confusion with builtin:trim or even reducing the other end of the string. There might be some confusion around _CHOP, but at least the connection to sv_chop is there.

tonycoz · 2024-12-02T04:42:04Z

Maybe OP_SUBSTR_LEFT borrowing from perl's BASIC antecedents.

peep.c

richardleach · 2024-12-05T14:28:27Z

Maybe OP_SUBSTR_LEFT borrowing from perl's BASIC antecedents.

Ok, that seems straightforward enough without colliding with Perlspace. Will rename.

Variants are named to match the style of macros in op.h

richardleach · 2024-12-12T22:13:22Z

OP renamed to OP_SUBSTR_LEFT and, I think, review comments all addressed.

peep.c

pp.c

This commit adds OP_SUBSTR_LEFT and associated machinery for fast handling of the constructions: substr EXPR,0,LENGTH,'' and substr EXPR,0,LENGTH Where EXPR is a scalar lexical, the OFFSET is zero, and either there is no REPLACEMENT or it is the empty string. LENGTH can be anything that OP_SUBSTR supports. These constraints allow for a very stripped back and optimised version of pp_substr. The primary motivation was for situations where a scalar, containing some network packets or other binary data structure, is being parsed piecemeal. Nibbling away at the scalar can be useful when you don't know how exactly it will be parsed and unpacked until you get started. It also means that you don't need to worry about correctly updating a separate offset variable. This operator also turns out to be an efficient way to (destructively) break an expression up into fixed size chunks. For example, given: my $x = ''; my $str = "A"x100_000_000; This code: $x = substr($str, 0, 5, "") while ($str); is twice as fast as doing: for ($pos = 0; $pos < length($str); $pos += 5) { $x = substr($str, $pos, 5); } Compared with blead, `$y = substr($x, 0, 5)` runs 40% faster and `$y = substr($x, 0, 5, '')` runs 45% faster.

As suggested in Perl#22785

tonycoz · 2024-12-17T22:48:31Z

pp.c

+
+    if (SvROK(sv) && do_chop) {
+        Perl_ck_warner(aTHX_ packWARN(WARN_SUBSTR),
+                        "Attempt to use reference as lvalue in substr"
+        );
+    }
+
+    if (do_chop) {
+        SvGETMAGIC(sv);
+        tmps = SvPV_force_nomg(sv, curlen);
+    } else
+        tmps = SvPV_const(sv, curlen);


You're checking SvROK(sv) before calling SvGETMAGIC() which would be incorrect if sv is tied - I think I would have caught that before, but maybe I missed it.

You could move the reference check after the SvGETMAGIC() inside the do_chop conditional to fix it (and better match pp_substr).

bulk88 · 2024-12-19T10:35:20Z

BINOPs like PP

if(index($str, 'ZZZZZZ) == -1) { }
XS have no concpext of "G_BOOL" content. There is definently a need to deliver bool contet, from runloop to the XS.

iabyn · 2024-12-23T10:29:15Z

On Thu, Dec 19, 2024 at 02:35:50AM -0800, bulk88 wrote: BINOPs like PP `` if(index($str, 'ZZZZZZ) == -1) { } `` XS have no concpext of "G_BOOL" content. There is definently a need to deliver bool contet, from runloop to the XS.

What has this got to do with the proposed OP_SUBSTR_LEFT op?

…

-- "You may not work around any technical limitations in the software" -- Windows Vista license

richardleach force-pushed the hydahy/op_substr_nibble2 branch from d6f958e to f258d43 Compare November 26, 2024 14:23

leonerd reviewed Nov 26, 2024

View reviewed changes

peep.c Outdated Show resolved Hide resolved

peep.c Outdated Show resolved Hide resolved

pp.c Outdated Show resolved Hide resolved

richardleach force-pushed the hydahy/op_substr_nibble2 branch from f258d43 to 98d187d Compare November 28, 2024 07:14

github-actions bot added the hasConflicts label Nov 28, 2024

richardleach changed the title ~~OP_SUBSTR_NIBBLE - a specialised OP_SUBSTR variant~~ OP_SUBSTR_CHOP - a specialised OP_SUBSTR variant Nov 28, 2024

richardleach force-pushed the hydahy/op_substr_nibble2 branch from 98d187d to 24108a7 Compare November 29, 2024 00:50

jkeenan removed the hasConflicts label Nov 30, 2024

tonycoz reviewed Dec 2, 2024

View reviewed changes

peep.c Outdated Show resolved Hide resolved

github-actions bot added the hasConflicts label Dec 4, 2024

richardleach force-pushed the hydahy/op_substr_nibble2 branch from 24108a7 to 2712d8f Compare December 5, 2024 22:23

Add MAXARG3 for neater checking of small argument counts

135a111

Variants are named to match the style of macros in op.h

richardleach force-pushed the hydahy/op_substr_nibble2 branch from 2712d8f to 3c55fa6 Compare December 5, 2024 22:36

github-actions bot removed the hasConflicts label Dec 6, 2024

richardleach changed the title ~~OP_SUBSTR_CHOP - a specialised OP_SUBSTR variant~~ OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant Dec 12, 2024

richardleach force-pushed the hydahy/op_substr_nibble2 branch from 3c55fa6 to c468cc5 Compare December 12, 2024 22:12

tonycoz reviewed Dec 16, 2024

View reviewed changes

peep.c Outdated Show resolved Hide resolved

tonycoz reviewed Dec 16, 2024

View reviewed changes

pp.c Outdated Show resolved Hide resolved

richardleach added 3 commits December 16, 2024 22:58

Use new MAXARG3 macros for readability

81f36e0

pp_substr: tweak len_is_uv assignment for readability

0e93328

As suggested in Perl#22785

richardleach force-pushed the hydahy/op_substr_nibble2 branch from c468cc5 to 0e93328 Compare December 16, 2024 23:13

tonycoz reviewed Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant #22785

OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant #22785

richardleach commented Nov 26, 2024

leonerd left a comment

richardleach commented Nov 26, 2024

richardleach commented Nov 26, 2024

richardleach commented Nov 27, 2024

richardleach commented Nov 28, 2024

leonerd commented Nov 28, 2024

richardleach commented Nov 28, 2024

jkeenan commented Nov 29, 2024

leonerd commented Nov 29, 2024

Grinnz commented Nov 30, 2024

richardleach commented Dec 1, 2024

tonycoz commented Dec 2, 2024

richardleach commented Dec 5, 2024

richardleach commented Dec 12, 2024

tonycoz Dec 17, 2024

bulk88 commented Dec 19, 2024

iabyn commented Dec 23, 2024 via email

OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant #22785

Are you sure you want to change the base?

OP_SUBSTR_LEFT - a specialised OP_SUBSTR variant #22785

Conversation

richardleach commented Nov 26, 2024

leonerd left a comment

Choose a reason for hiding this comment

richardleach commented Nov 26, 2024

richardleach commented Nov 26, 2024

richardleach commented Nov 27, 2024

richardleach commented Nov 28, 2024

leonerd commented Nov 28, 2024

richardleach commented Nov 28, 2024

jkeenan commented Nov 29, 2024

leonerd commented Nov 29, 2024

Grinnz commented Nov 30, 2024

richardleach commented Dec 1, 2024

tonycoz commented Dec 2, 2024

richardleach commented Dec 5, 2024

richardleach commented Dec 12, 2024

tonycoz Dec 17, 2024

Choose a reason for hiding this comment

bulk88 commented Dec 19, 2024

iabyn commented Dec 23, 2024 via email